Skip to content

ci: fix and deduplicate rank_gene_groups tests#3966

Merged
flying-sheep merged 4 commits into
mainfrom
fix-rgg
Feb 6, 2026
Merged

ci: fix and deduplicate rank_gene_groups tests#3966
flying-sheep merged 4 commits into
mainfrom
fix-rgg

Conversation

@flying-sheep
Copy link
Copy Markdown
Member

@flying-sheep flying-sheep commented Feb 5, 2026

This gets rid of potential numpy warnings by avoiding pickles in our test files.

What I did:

  1. converted test results from recarrays to regular arrays, and converted the “names” arrays to np.uint8s (manually checked that everything worked, it’s 40 numbers × 2 test methods
  2. saved to npz with allow_pickle=False, which is now possible, and deleted the old files
  3. restructured the tests without changing the logic. no clue why [:7] is done for wilcoxon in the first test and for both in the second 🤷

@flying-sheep flying-sheep added this to the 1.12.1 milestone Feb 5, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.26%. Comparing base (4c7c5c0) to head (6f7cacf).
⚠️ Report is 62 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3966   +/-   ##
=======================================
  Coverage   78.26%   78.26%           
=======================================
  Files         117      117           
  Lines       12633    12634    +1     
=======================================
+ Hits         9887     9888    +1     
  Misses       2746     2746           
Flag Coverage Δ
hatch-test.low-vers 77.60% <100.00%> (+<0.01%) ⬆️
hatch-test.pre 77.22% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/scanpy/metrics/_metrics.py 97.18% <100.00%> (+0.04%) ⬆️

Comment on lines +82 to +83
mtx = mtx.astype(np.float64)
np.divide(mtx, sums, where=sums != 0, out=mtx)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

expect unitialized memory in output i.e., from the warning in the "failing" test, I would think out= should have the result of np.empty, not mtx. Or am I misinterpreting?

Copy link
Copy Markdown
Member Author

@flying-sheep flying-sheep Feb 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After my change, what we do is to leave all values in mtx where sums==0 as they are (and divide the others):

mtx[sums != 0] /= sums

Before my change we replaced all values in mtx where sums==0 with uninitialized memory (and divide the others):

mtx_new = np.empty(mtx.shape)
mtx[sums != 0] = mtx_new / sums

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow! "If not provided or None, a freshly-allocated array is returned." from the numpy docs on divide - I didn't realize this was default behavior to return uninitialized memory. Ok then!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, some nuts behavior. Good they have that warning now.

Comment on lines +82 to +83
mtx = mtx.astype(np.float64)
np.divide(mtx, sums, where=sums != 0, out=mtx)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow! "If not provided or None, a freshly-allocated array is returned." from the numpy docs on divide - I didn't realize this was default behavior to return uninitialized memory. Ok then!

@flying-sheep flying-sheep merged commit 76b625e into main Feb 6, 2026
14 checks passed
@flying-sheep flying-sheep deleted the fix-rgg branch February 6, 2026 16:28
meeseeksmachine pushed a commit to meeseeksmachine/scanpy that referenced this pull request Feb 6, 2026
flying-sheep added a commit that referenced this pull request Feb 6, 2026
…_groups tests) (#3967)

Co-authored-by: Philipp A <flying-sheep@web.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integration Testing CI Test Failure on python 3.13 with prerelease dependencies

2 participants